we need to replace that function with something that isn't specifically designed for the cartesian region. ultimately we want to provide a binwise_counts function on the region class. so i would add a function to the quadtree class called get_binwise_counts() that accepts a list of lons and lats and returns in the counts corresponding to the polygons in the quadtree class.

also, it looks like the build is failing because we haven't specified mercantile in the requirements.txt and requirements.yaml files in the top-level directory.

can you also explain a bit how this package called area works to calculate the area given lat/lon points? maybe this would be good to put in some documentation about this class.

Thank you for your comments.

I have a question. I understand that I need to write two different functions that say, e.g. "get_spatial_counts" and "get_spatio_magnitude_counts". However, I am not entirely sure where to put these functions? Should it be a part of Catalog class or QuadtreeGrid2D class? I guess in current implementation a wrapper function is called from catalog class. What is your opinion?

good question. now that im thinking about it more, i think the best would be to modify the function in the catalog class to work with the region object directly. right now it calls this _bin_catalog_spatial_counts function, instead of working directly with the region object. once we implement this behavior, we can just ditch these _bin_catalog_spatial_counts functions.

the general procedure of the function would be to use the get_index_of() function of the region class to get the indexes and then increment the counts of a numpy.array(). we can then use the bin1d_vec() function to compute the magnitude index (because these are regularly spaced). basically, we just need to take the function that exists and make it more general to work with any region object that implements the get_index_of() function.

I have identifed an issue with bin1d_vec() function. I am not sure whether this is actually an issue or not. it takes in mbins and magnitude and provides the index of the bin, where magnitude lies.

from csep.utils.calc import bin1d_vec
mbins = numpy.arange(5.95, 9, 0.1) #This gives bins from 5.95 to 8.95
idx = bin1d_vec(mm, mbins, tol=0.00001, right_continuous=True)

so it returns the magnitude bin of mm.
if mm is actually lying inside a mbins, it would return its bin.

idx = bin1d_vec(6, mbins, tol=0.00001, right_continuous=True) #This would give 0: Which is fine.

But if, I give some magnitude that is below the mbins, e.g. mm=4 or 5. It would start negative counting beyond 5.95.

idx = bin1d_vec(5, mbins, tol=0.00001, right_continuous=True)
this gives -10

idx = bin1d_vec(4, mbins, tol=0.00001, right_continuous=True)
this gives -20

Ideally, it should be returning zero. Is it something you desired? or is it a bug?

this expected behavior is that if the value falls outside the range it returns -1. the downside is that this is a valid index in python, and some issues arised from returning None there. the fact its returning -10 is not expected, but practically should not cause any issues it should return -1. this only happens with the right_continuous flag.

this issue only affects scalarr values, if you were to test with an array the -1 should be returned as expected. ill submit a pr to fix this in the next bit.

This may cause problem for the following line. Because -1 or -10 are valid indices. So we may need additional check, whether catalog is filtered for the minimum magnitude or not.

numpy.add.at(out, idx, 1)

yes, you should check whether the bins values are correct. you will notice both of the spatial counts functions have some sort of check inside them. these checks also account for potentially masked cells. see the _bin_catalog_spatial_counts function.

we should probably add a check to the magitude_counts function in the catalog class though. i think that is maybe where you found that line of code.

I added the check to spatial_count and spatial_magnitude_count functions. So now it should work alright.

I wanted to know, how do you treat the right corner event? I mean, the earthquakes occurring at lon of 180 or lat of 90?

Now, I have completed integration of quadtree grid into pyCSEP. It is a basic working version for global testing region.
Please have a look at the code, and let me know if there are any suggetions.

Next steps are:
-Improve the implementation of some functions
-Integrate California quadtree grid

great, lets go over it on the call tomorrow! can you also make sure to add mercantile (and any other extra requirements) to the requirements.txt and requirements.yaml files? before ill be able to merge, the build will need to be successful.

I added the check to spatial_count and spatial_magnitude_count functions. So now it should work alright.
great!

I wanted to know, how do you treat the right corner event? I mean, the earthquakes occurring at lon of 180 or lat of 90?

the point at 180 should map to the bin at -180, but we should probably make an additional check, maybe in the constructor of the catalog class. this would enforce a convention about how lat/lon are reported by a catalog instead of trying to assume how to handle these points during binning.

i'm not sure how exact we want to handle a lat of 90, because technically that is a point. conceptually, i think if an earthquake were to occur directly at lat = 90, we would want to place it in the [89.9, 90.0) bin.

what do you think?

I personally would like to go with the first suggestion. The point on 180 should belong to -180, both are same in actual. I would prefer to make this a constructor in Catalog class. May be change 180 to -180. What would you say?

I added requirements of mercantile and area packages. But its still not resolving the issue of building package.
I am not sure why.

it seems to be breaking during the cartopy build. yoou can look at the error message from the failing build by going here and looking through the terminal output from the build. i took a look and it seems like a cartopy issue, but take a look and let me know what you think.

also, it looks like the area package isn't available on conda-forge. either we need to make a feedstock for this package (which might be more work than we want at the moment; although id be happy to help you set this up if you'd like to maintain the feedstock) or we should implement this functionality into the package. it only seems like we are using it to compute the area, which could probably be done with the pyproj function that is already implemented. i think the best way forward will be to implement an area function in regions module.

I have a question. Would that be ethical and a good idea to directly copy the implementation of area from package to our implementation? If that is alright, then I can simply do that. Or if its a problem, then I would think of something else. The implementation is available here:

https://github.com/scisco/area

that's a good question. from a legal standpoint, it depends on the license of the software package. i think that bsd-2 allows us to do this, see the license here. id recommend we write our own area calculation and associated unit-tests. im not normally about re-inventing the wheel, but this is a standad and easy enough to implement, we should probably do it.

looking at their code, it seems like something we could easily implement as well. i tagged you in a pull request from @hanbao-ucla. i think we should implement a get_area function on the region class. i think you already wrote this, but left it as a stand-alone function in the regions module. this function would return a numpy array where each element contains the area of the corresponding polygon.

what do you think?

Yeah that would be neat, as sometimes we need both 'rate [eg., eq/day/cell or eq/year/Earth]' and 'rate_density [eg. eq/year/km^2]'

Hi @wsavran
I feel that the code build is failing due to following erros.
Can you guide me with this error?

from csep.core.regions import CartesianGrid2D, Polygon, create_space_magnitude_region
E ImportError: cannot import name 'CartesianGrid2D' from 'csep.core.regions' (/home/runner/work/pycsep/pycsep/csep/core/regions.py)

from csep.core.regions import QuadtreeGrid2D
E ImportError: cannot import name 'QuadtreeGrid2D' from 'csep.core.regions' (/home/runner/work/pycsep/pycsep/csep/core/regions.py)

I wrote quadtree forecast reader in csep.utils.reader. Thus I needed to call QuadtreeGrid2D from csep.core.regions in readers, which caused circular imports. So I tried to rectify this by calling QuadtreeGrid2D inside the reader function. For now, I did this, as we agreed, but lets discuss sometime to improve structure of forecast reading. As the forecast handling remains same in general. We just have to call a different region,
region = QuadtreeGrid2D.fromquadkeys(qk), instead of
region = CartesianGrid2D([Polygon(bbox) for bbox in bboxes], dh, mask=poly_mask).

The forecast handling remains same overall. I believe forecast reading can also be done in the previous same function that you wrote, by introducing an if/else statement.

As for documentation, I wrote documentation in docs/concepts/forecasts.rst and docs/concepts/regions.rst.

I can not build them on my PC. I am not sure whether I was doing it the correct way or not. But I am sure, if you accept the changes in the docs and run "make html", it should work.

hi @khawajasim, i got my github 2fa issues sorted and im back online. ill test out the build tonight and we can chat about it on the call tomorrow. thanks for spending some time to write the documentation.

after more than enough time, im happy to say that im merging this pull request into the v0.6.0 release candidate branch